Add preview metadata to buffered tail traces#6375
Add preview metadata to buffered tail traces#6375Ankcorn wants to merge 3 commits intocloudflare:mainfrom
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
src/workerd/io/trace.c++
Outdated
|
|
||
| TraceEventInfo::TraceItem::TraceItem(rpc::Trace::TraceEventInfo::TraceItem::Reader reader) | ||
| : scriptName(kj::str(reader.getScriptName())) {} | ||
| : scriptName(reader.hasScriptName() ? kj::Maybe<kj::String>(kj::str(reader.getScriptName())) |
There was a problem hiding this comment.
Opencode drive by changed this because it thinks the old implementation has a bug...
IMO seems suss and I'm going to revert this line but hello @danlapid I just wanted to flag it to you incase opencode smarter than me
There was a problem hiding this comment.
I think it's right. The previous code didn't preserve the emptiness of the scriptName capnp field during a serialization round trip (which seems to be used by TraceCustomEvent::sendRpc method for sending traces over an RPC interface).
I think this matters in practice too since the worker may be deployed without a script name in some cases. I suspect the failures you're seeing on the internal build are due to this changing behaviour in cases where script name is empty. It likely results in a trace item having a null scriptName instead of empty string. We might need to put the change behind a compatibility date if so, because even though it's a bug fix it's a change in API behaviour.
src/workerd/api/trace.h
Outdated
| jsg::Optional<ScriptVersion> getScriptVersion(); | ||
| jsg::Optional<kj::StringPtr> getDispatchNamespace(); | ||
| jsg::Optional<kj::Array<kj::StringPtr>> getScriptTags(); | ||
| jsg::Optional<jsg::Dict<kj::String>> getPreview(); |
There was a problem hiding this comment.
I think we should make this a JSG struct instead of a JSG dict. I think dicts are meant to be for when you don't know the keys at compile time. See getScriptVersion for inspiration.
Ankcorn
force-pushed
the
tankcorn/preview-observability
branch
from
fb12ef3 to
4dddab3
Compare
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
2 participants
Summary
Add preview metadata to buffered tail traces.
This exposes:
to classic/buffered tail workers.
Why Why
edgeworker needs to surface pipeline-level preview identity in classic tail worker
observability.
That requires workerd to support carrying preview metadata through the buffered trace path;
otherwise classic tail workers only receive traces without any preview context.
Changes
Notes
This only adds the workerd support layer. The corresponding edgeworker change is what populates
this field from pipeline preview metadata.